Skip to content

Add --allow-build flag & tool.pywrangler setting#144

Open
hoodmane wants to merge 1 commit into
cloudflare:mainfrom
hoodmane:allow-build
Open

Add --allow-build flag & tool.pywrangler setting#144
hoodmane wants to merge 1 commit into
cloudflare:mainfrom
hoodmane:allow-build

Conversation

@hoodmane

@hoodmane hoodmane commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

By default we pass --no-build because building a binary Pyodide wheel will fail in a confusing way. This makes it annoying to work with local packages though. This adds a --allow-build setting to override.

By default we pass `--no-build` because building a binary Pyodide wheel will
fail in a confusing way. This makes it annoying to work with local packages
though. This adds a `--allow-build` setting to override.
source = pkg.get(key)
if not isinstance(source, dict):
return False
# A local reference has a `path``

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# A local reference has a `path``
# A local reference has a `path`

Comment on lines +118 to +120
When *allow_build* is True, ``--no-build`` is omitted so source
distributions and local directory sources may be built. Building a binary
extension in this way will fail in a confusing way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment already exists inside _compile_lockfile so I don't think it has to be duplicated here.

Comment on lines +64 to +66
# ---------------------------------------------------------------------------
# Flag / config propagation into sync()
# ---------------------------------------------------------------------------

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these tests doesn't add much value. Also, these tests are quite easy to break whenever we update internal logics or parameters.



@pytest.mark.parametrize("allow_build", [True, False])
def test_compile_lockfile_no_build_toggle(tmp_path, allow_build):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too. We are adding too many mocks just to test

if not allow_build:
  args.append("--no-build")

which is pretty obvious.

Of course, this kind of test would be great to prevent any kind of regression we make by accident later, but I think adding 4 mock functions just to test this is a code smell. We should our refactor our logic to make the test easier.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing a real, e2e test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants